docs(idl): three documented config fields are not honored - #835

No actionable comments were generated in the recent review. 🎉

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

That failure was mine, and it is fixed in 0d3667b.

The gate is Require an idl/VERSION bump when the schema changed:

##[error]the .proto surface changed but idl/VERSION is still 1.2.0.

I had reasoned about SCHEMA_LOCK in the PR body but missed that the same digest also drives a second, separate rule. schema_lock.sh spells out the scale, and it covers this case explicitly:

# IDL_VERSION (idl/VERSION) is hand-maintained semver for the schema surface:
#   patch = comments/docs only · minor = additive · major = wire-breaking

So a comment-only change is a patch bump, not an exemption. idl/VERSION is now 1.2.1 and SCHEMA_LOCK carries it.

Ran the exact command CI runs, against main's copy of the lock as the base:

$ git show origin/main:idl/SCHEMA_LOCK > /tmp/base_lock
$ idl/codegen/schema_lock.sh --check --require-bump /tmp/base_lock
✓ idl/SCHEMA_LOCK current: 1.2.1 / 6fe2a85a... (40 protos)
exit=0

And confirmed the bump is what fixes it rather than assuming, by putting idl/VERSION back to 1.2.0 and re-running:

Bump idl/VERSION — patch for comments only, minor for additive
changes, major for anything wire-breaking — then re-run
./idl/codegen/generate_all.sh so SCHEMA_LOCK follows.

The digest itself is unchanged from the first push (6fe2a85a...), since the proto bytes did not move; only IDL_VERSION did. IDL_PROTO_COUNT is still 40, and nothing else in the tree pins 1.2.0.

The offer from the PR body still stands: if you would rather SCHEMA_LOCK only ever move through a full generate_all.sh on a machine with the complete toolchain, say so and I will drop both lock hunks and let you regenerate.

Widened rather than opened as a second proto PR, for a concrete reason: any .proto edit moves IDL_SCHEMA_SHA256 and needs the idl/VERSION bump, so two open proto PRs would collide on SCHEMA_LOCK and VERSION and whichever landed second would need a rebase. One patch bump covers both.

The new one is VoiceAgentComposeConfig.instructions, and it is a stronger case than the two network overrides. Its documentation makes a definite functional claim:

System prompt for the agent. Governs persona AND spoken delivery ("talk quickly", "sound warm"), not just content. Same name and role as OpenAI Realtime session.instructions. Unset uses the commons voice default (short, spoken, no markdown).

This is the only system prompt the voice path reads: llm_generation.system_prompt is IGNORED here.

Nothing reads it. config_from_proto (voice_agent_internal_helpers.cpp:415) translates this message into rac_voice_agent_config_t and consumes 7 of its 11 fields; instructions is not among them, and grep for .instructions() across core/src returns nothing outside generated code.

What every turn actually gets is a compile-time constant:

// voice_agent_internal_helpers.cpp:76
rac_llm_options_t make_voice_llm_options() {
    rac_llm_options_t options = RAC_LLM_OPTIONS_DEFAULT;
    ...
    options.system_prompt = kVoiceAgentSystemPrompt;

So "unset uses the commons voice default" understates it: the default is what you get whether or not you set the field. The last line of that comment is the part worth correcting, because a reader takes it as "set instructions, not llm_generation.system_prompt", and the true statement is that the voice path reads neither.

llm_generation is inert for the same reason and is now noted too: make_voice_llm_options() takes no argument, so nothing on that message reaches the LLM.

I did not try to wire instructions up. It would need a slot on rac_voice_agent_llm_config_t, which has only model_path / model_id / model_name, so honoring it is a public C ABI change plus threading the value to three make_voice_llm_options() call sites across voice_agent.cpp, voice_agent_proto_abi.cpp and voice_agent_d7_abi.cpp. That is a feature decision and not one I should make unasked, and I cannot exercise the voice path here to prove such a change behaves.

turn_detection is also unread by config_from_proto, but unlike the other three I could not convince myself the message has no other consumer, so I left it alone rather than assert something I had not established.

Lock and version handling is unchanged from the original: IDL_VERSION stays 1.2.1, the digest moves to 0eed7068..., and CI's own check passes.

$ idl/codegen/schema_lock.sh --check --require-bump <main's SCHEMA_LOCK>
✓ idl/SCHEMA_LOCK current: 1.2.1 / 0eed7068... (40 protos)

Replying to this comment

That makes sense -- thanks for flagging it! The schema_lock.sh --update approach you used is exactly right here since this is a comment-only change to the schema; no need to regenerate via the full generate_all.sh toolchain for this one. Keeping your SCHEMA_LOCK diff as-is.

Reviewed with help from Claude Code and Codex.